Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow module without version #600

Closed
wants to merge 16 commits into from

Conversation

Gijsreyn
Copy link
Contributor

PR Summary

Allow modules that don't have a version directory to be added in $env:PSModulePath.

PR Context

The Get-DscResourceModules function scans the $env:PSModulePath to find relevant modules to be included in the cache. When a module doesn't have a version directory, it does not find the module. This change allows the ability to include modules that don't contain the directory.

I added this change because it can be useful for testing purposes to see the participation of DSC resources when developing them. For example, take the winget-dsc repository. Simply adding the resource to the environment variable adds it for participation.

@Gijsreyn
Copy link
Contributor Author

@SteveL-MSFT I know you have a ton on your plate, but hopefully you have some time to check out this small change. If you want me to add a test, I can make a duplicate of the TestClassResource. Thanks.

Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test

@Gijsreyn Gijsreyn requested a review from SteveL-MSFT November 27, 2024 07:53
@Gijsreyn
Copy link
Contributor Author

Tests have been included. If you need it in the config, let me know, thought I don't believe it is needed.

@Gijsreyn Gijsreyn force-pushed the allow-module-without-version branch from 9767de9 to 8edeb32 Compare December 7, 2024 20:05
@Gijsreyn Gijsreyn force-pushed the allow-module-without-version branch from 1a488e3 to 58b9d3d Compare December 9, 2024 18:12
@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Dec 9, 2024

Added the OTBS to the settings.json file. It might help other folks in the future when they are working with the PowerShell scripts. All remarks resolved @SteveL-MSFT

Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, waiting on Andrew to review

@Gijsreyn Gijsreyn requested a review from anmenaga December 9, 2024 18:51
# Get-Module -ListAvailable is slow and has a bug causing incorrect reporting
$moduleFolders = Get-ChildItem $folder -Directory
if (-not $moduleFolders) {
$moduleFolders = Get-ChildItem (Split-Path $folder -Parent) | Where-Object { $_.Name -eq (Split-Path $folder -Leaf) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -Depth 2 parameter in original code was supposed to handle both cases: modules with version subdirs and without.
Can you please give an example directory/module structure that was not handled correctly by old code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When working in the winget-dsc repository, I often found myself doing something like:

$env:PSModulePath += ";C:\source\winget-dsc\resources\Microsoft.DotNet.Dsc"

# Call function
Get-DscResourcesModules
# Old code did not list out the module

Hope that sheds some light on it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please post an output of
Get-ChildItem -Recurse -Path C:\source\winget-dsc\resources\Microsoft.DotNet.Dsc | %{$_.FullName}
Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There you go:

image

I have added both. The left is the main branch, and the right is the current one in this pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you; in the right picture - what is the full path to NpmDsc.psd1 that is returned by Get-DSCResourceModules ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I got what's happening.
The module discovery in PowerShell (and consequently DSC PS adapter) requires that all module files should be in the module directory; and $env:PSModulePath should point to the parent of that module directory.
In your case, the line should be:
$env:PSModulePath += ";C:\source\winget-dsc\resources"
rather than
$env:PSModulePath += ";C:\source\winget-dsc\resources\NpmDsc".
This should work with existing code. Please try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, Andrew. I tried it in the past, and that works. But sometimes, I want only particular modules to be found, with the structure mentioned in this PR and in the test. What are your thoughts on those?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gijsreyn I think the problem is that this is contrary to how module discovery works with PS7 and it would be preferable to not deviate from that. Is this only for development purposes or do you have a real scenario where you want to limit the discovery?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can understand, and this was the use case. Feel free to abandon the pull request. I appreciated the time you guys looked into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @SteveL-MSFT. After testing for a while, I got a scenario that breaks the current testing cycle.

Imagine the following use case found while developing in winget-dsc. To test out both cases by either using Invoke-DscResource or the class directly, we declare at the top of the test the using module statement.

When modules are being discovered in the lower levels, so say C:\source\winget-dsc\resources\NpmDsc, it correctly loads the using module statement, but it doesn't do it for PSDesiredStateConfiguration. If I now add it on higher level in the same call, I get an error stating:

PS C:\Users\User> $env:PSModulePath += ";C:\source\winget-dsc\resources\Microsoft.Windows.Setting.Language"
PS C:\Users\User> Get-DscResource
Exception: Exception setting "Module": "Cannot convert the "System.Object[]" value of type "System.Object[]" to type
"System.Management.Automation.PSModuleInfo"."

I know you've closed down the ticket, but perhaps we can still consider it. Thanks.

@Gijsreyn Gijsreyn requested a review from anmenaga December 12, 2024 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants